Skip to content

[9.0][ADD] base_kanban_stage: Stage model and abstract logic - #679

Merged
dreispt merged 5 commits into
OCA:9.0from
LasLabs:feature/9.0/LABS-134-create-medical_base_stage-object
Jan 6, 2017
Merged

[9.0][ADD] base_kanban_stage: Stage model and abstract logic#679
dreispt merged 5 commits into
OCA:9.0from
LasLabs:feature/9.0/LABS-134-create-medical_base_stage-object

Conversation

@tedsalmon

Copy link
Copy Markdown
  • Add Kanban-compatible stage model base.kanban.stage
  • Add views, menu items, actions, and access controls needed to manage
    base.kanban.stage records
  • Add abstract model base.kanban.abstract that other models can inherit from to
    gain Kanban stage functionality
  • Add base Kanban view base_kanban_abstract_view_kanban, which can be
    customized as needed for use with models that inherit from base.kanban.abstract
  • Add model base.kanban.abstract.tester, which is needed for
    web.kanban.abstract unit tests

This is a repo change based upon the discussions in PR OCA/web#490 (comment)
cc @lasley @obulkin

@tedsalmon
tedsalmon force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from 7816cdf to 7d22168 Compare December 28, 2016 18:58
@lasley lasley changed the title [ADD] base_kanban_stage: Stage model and abstract logic [9.0][ADD] base_kanban_stage: Stage model and abstract logic Dec 28, 2016
@lasley lasley added this to the 9.0 milestone Dec 28, 2016
@lasley

lasley commented Dec 28, 2016

Copy link
Copy Markdown
Contributor

@t3ddftw - we lost @obulkin attribution in this. Do the following to fix:

  • Squash Oleg's commits in the previous branch, force push
  • Add LasLabs/web as a remote to your server-tools clone named as laslabs-web
  • Change to this branch, git reset --hard OCA/9.0 (assuming the OCA remote is named OCA in your local), don't push
  • git remote update && git cherry-pick HASH_FROM_SQUASHED
  • Resolve conflicts if any, commit, don't push
  • git cherry-pick 7d2216804a8cd710eb13c0ce629f741f45c93b62
  • Resolve conflicts, commit, force push after verifying no disaster
  • Delete the laslabs-web remote
  • Delete the branch in LasLabs/web (use github interface so it's recoverable just in case)

@tedsalmon
tedsalmon force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from 7d22168 to 540a97a Compare December 28, 2016 20:01
@tedsalmon

Copy link
Copy Markdown
Author

@lasley,

Done! :)

@lasley lasley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @t3ddftw - few comments, but otherwise was reviewed in the previous PR


@api.model
def _default_stage_id(self):
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add # pragma: no cover

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment thread base_kanban_stage/README.rst Outdated

* Dave Lasley <dave@laslabs.com>
* Oleg Bulkin <obulkin@laslabs.com>
* Daniel Reis

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreispt - what email did you want here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use dreis.pt@hotmail.com

@tedsalmon
tedsalmon force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from e0ba631 to db936d8 Compare December 28, 2016 23:52
@tedsalmon
tedsalmon force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from f4d841f to cd0fb9b Compare December 29, 2016 23:30

@dreispt dreispt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great module!
Design looks good.
On the implementation I added a few questions.

Comment thread base_kanban_stage/README.rst Outdated

* Dave Lasley <dave@laslabs.com>
* Oleg Bulkin <obulkin@laslabs.com>
* Daniel Reis

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use dreis.pt@hotmail.com

Comment thread base_kanban_stage/__openerp__.py Outdated
'author': "LasLabs, Odoo Community Association (OCA)",
'category': 'base',
'depends': [
'web_kanban',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather replace this with a dependency on base, since this module does not modify or extend the web_kanban features.

'''Inherit from this class to add support for Kanban stages to your model.
All public properties are preceded with kanban_ in order to isolate from
child models, with the exception of stage_id, which is a required field in
the Kanban widget and must be defined as such.'''

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP257 prescribes: For consistency, always use """triple double quotes"""

' stage and with the same priority',
)
kanban_priority = fields.Selection(
selection=[('0', 'Normal'), ('5', 'Medium'), ('10', 'High')],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reasoning behind this priority scale?
I'm curious why not sticking to the scales using by official modules: 0,1 or 0,1,2.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@obulkin - Could you please comment on this?

Thanks!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of a question for @lasley. I just used the scale that he introduced in https://github.com/OCA/vertical-medical/pull/121/files#diff-a9f682d57f72b368fcf819ed09e1f3c1R27.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just made that stuff up, go ahead change it as @dreispt suggested

default=lambda s: s._default_stage_id(),
domain=lambda s: [('res_model.model', '=', s._name)],
)
kanban_user_id = fields.Many2one(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use user_id directly? AFAIK it is the field name expected for the responsible person (in places like the mail module logic).

def _read_group_stage_ids(
self, domain=None, read_group_order=None, access_rights_uid=None
):
stage_model = self.env['base.kanban.stage'].sudo(access_rights_uid)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If access_rights_uid is an optional argument, we should only use sudo if it is provided.


<record id="my_model_view_kanban" model="ir.ui.view">
<field name="name">My Model - Kanban View</field>
<field name="model">my.model</field>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add name="mode">primary< & a note indicating that this must be set in order for the proper inheritance. Per the docs:

mode
inheritance mode, see Inheritance. If inherit_id is unset the mode can only be primary. If inherit_id is set, extension by default but can be explicitly set to primary

@oca-clabot

Copy link
Copy Markdown

Hey @tedsalmon, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@lasley

lasley commented Jan 3, 2017

Copy link
Copy Markdown
Contributor

@tedsalmon used to be @t3ddftw and is covered under the @LasLabs CLA. Can someone update please and thanks?

@tedsalmon
tedsalmon force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from 8b27e7e to 693c1d4 Compare January 3, 2017 21:36
@tedsalmon

Copy link
Copy Markdown
Author

@dreispt,

Updates made as requested :).

Failing build is not caused by any changes in this PR, FWIW.

Thanks!

obulkin and others added 5 commits January 3, 2017 13:54
* Add Kanban-compatible stage model web.kanban.stage
* Add views, menu items, actions, and access controls needed to manage
web.kanban.stage records
* Add abstract model web.kanban.abstract that other models can inherit from to
gain Kanban stage functionality
* Add base Kanban view web_kanban_abstract_view_kanban, which can be
customized as needed for use with models that inherit from web.kanban.abstract
* Add model web.kanban.abstract.tester, which is needed for
web.kanban.abstract unit tests
* Add Kanban-compatible stage model base.kanban.stage
* Add views, menu items, actions, and access controls needed to manage base.kanban.stage records
* Add abstract model base.kanban.abstract that other models can inherit from to gain Kanban stage functionality
* Add base Kanban view base_kanban_abstract_view_kanban, which can be customized as needed for use with models that inherit from base.kanban.abstract
* Add model base.kanban.abstract.tester, which is needed for base.kanban.abstract unit tests
* Add pragma no cover to `None` return in method
* Centralize default_stage_id and return empty RecordSet of type `base_kanban_stage`
* Update depends to `base`
* Update priority selections to match Odoo guidelines
* Update comment block to triple double quotes
* Update `kanban_user_id` to `user_id` in code and view
* Update README to incorporate use of mode in example view
@lasley
lasley force-pushed the feature/9.0/LABS-134-create-medical_base_stage-object branch from 693c1d4 to 6f3c0c0 Compare January 3, 2017 21:54
@lasley

lasley commented Jan 3, 2017

Copy link
Copy Markdown
Contributor

Failing build is not caused by any changes in this PR, FWIW.

Rebased onto OCA/9.0

@dreispt

dreispt commented Jan 6, 2017

Copy link
Copy Markdown
Member

Failing testes seem unrelated (😢 )
Merging!

@dreispt
dreispt merged commit ac2e392 into OCA:9.0 Jan 6, 2017
@tedsalmon
tedsalmon deleted the feature/9.0/LABS-134-create-medical_base_stage-object branch January 6, 2017 19:01
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (8.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants